feat: remote environments for SSH and Docker execution targets (phase 1: creation-time binding) - #4062
feat: remote environments for SSH and Docker execution targets (phase 1: creation-time binding)#40627Sageer wants to merge 350 commits into
Conversation
…and retain decoder chunks ManagedRemoteRuntime.connect() bypassed connectInflight for connected views, so a reconnect of a previously-connected runtime never surfaced 'connecting'/whenReady and acquireWhenReady failed fast with runtime.unavailable instead of awaiting the reconnect. Route the connected-view branch through the same tracked path as the pending branch; on success the view settles on its inner's status so a view that joined another view's connect stays usable. LineFrameDecoder.push() re-concatenated the whole accumulated prefix with every chunk, costing O(n^2) copying for a large frame delivered in small transport chunks. Retain chunk views and copy only once when a frame completes; framing, boundary, and protocol-violation behavior is unchanged.
…repo-wide Runtime->Environment across agent-core-v2, remote-exec, node-sdk, kap-server, klient, acp-server, apps/kimi-code, apps/vis, apps/kimi-inspect and the en/zh docs: the Runtime interface and its family (registry, lease, binding, provider, unit host, workspace view), the [environments] config section and .kimi-code/environments.toml, the /environment slash command and --environment flag, the session environment REST/WS surface, and the SDK session methods. The Runtime.environment field becomes host. Durable wire surfaces break clean with no shims (the feature is experimental): the environment.set_binding event type, environment.status.changed, the runtimeBinding/runtime.binding state keys, the tui.toml status-line slot id, and the klient IPC service key. Unrelated runtime concepts (goal actor enum, MCP runtime names, model runtime config, worker runtimes, kimiCu, the remote_runtime experimental flag) keep their names.
Rename the leftovers the round-1 review enumerated: the REST error string and OpenAPI descriptions on the environment surface, the environment-provider error strings, the remaining type names (EnvironmentReadStreamSource, EnvironmentStdioTransport, the test wire types), execution-target locals and test fake variables, and revert the session-status variable to runtimeStatus to match its family.
Rename the createGeneration locals to environment (declaration + usages), switch three lease-value longhands to shorthand, and retitle the kap-server environment route describe block.
Rename the last batch of runtime-flavored factories, locals, params, and test titles across the test tree to environment (registries, binding, program, session, workspace, plan, media, kap-server, node-sdk, acp-server, remote-exec), switch the remaining lease-value longhands to shorthand, and keep the unrelated runtime zones (actor runtime, worker runtimes, FiberRuntime, kimiCu, MCP runtime names, model runtime config) intact.
Remove the remote_runtime experimental flag entirely: the flag definition, its KIMI_CODE_EXPERIMENTAL_REMOTE_RUNTIME env var, and every enabled() gate across the engine (provider attach, session manager, agent binding, workspace roots), the remote-exec provider, the node-sdk session surface, the kap-server REST routes, and the TUI (slash command, footer slot, mention suggester, feedback attachment). Remote environments are now unconditionally on. Also register the environment.status.changed event in the kap-server zod agentEventSchema so it enters the AsyncAPI contract, document the feature as stable in the en+zh guides and references (dropping flag instructions and experimental labels), complete the server-api environment endpoint docs with the declare route, and fold the five pending experimental changesets and the rename changeset into a single graduation changeset — the feature had no prior release, so this is its first user-facing announcement.
The trust prompt only listed gated MCP servers even though the SDK already returns gatedEnvironments with each declaration's full launch command line. An untrusted repo could declare launchers in .kimi-code/environments.toml and the prompt never showed what the user authorized. Render the launchers like the MCP targets, with the same control-character sanitization.
The remote-exec provider re-resolved declarations only on config-section changes and the project file watch. A workspace materialized while untrusted kept project environments unregistered after the user accepted trust, and a project default then seeded sessions with an id absent from the registry. Program now re-fires the local generation's trust changes on a stable onDidChangeTrust event (re-subscribed across generation rebuilds), the manager passes it through EnvironmentProviderContext, and the provider reconciles on every flip in both directions.
The node-sdk and kap-server each carried an identical unguarded read-merge-write of .kimi-code/environments.toml, so two concurrent adds (or one via REST plus one via SDK) read the same snapshot and the last write silently dropped the other entry. Consolidate the helper into agent-core-v2 with a per-path promise-chain mutex so updates serialize per file; both call sites now share it.
…ect-local config Two graduation-blocking finds from the live smoke: - The native print path bootstrapped the engine without ever attaching the remote-exec provider, so declared environments never registered and kimi -p --environment <id> always failed 'environment does not exist'. Attach the provider after bootstrap, mirroring the SDK client. - FileProjectLocalConfigService treated only Node errnos as path-missing, so a remote HostFsError (no Node-style cause) for a missing .kimi-code/local.toml crashed session startup with storage.io_failed instead of reading as no project-local config. Accept the fs-domain not-found codes too.
The flag was parsed and honored in print mode, but the interactive TUI's startup options never carried it and the lazy first session was created without environmentId — kimi --environment <id> silently started a local session. Thread it through TUIStartupOptions into the lazy creation path with the same first-session-only consumption as --agent.
…n the busy frame Container.invalidate() only clears child render caches; it never schedules a repaint, so a switch/reconnect/declare failure that resolved after the keypress cycle left the dialog stuck on 'Connecting…' until the next key. Thread requestRender through the three environment dialogs' options (the MigrationScreenComponent precedent), expose it on SlashCommandHost, and invoke it after every async setBusy/showError/setOptions. Also sweeps the stale '(experimental remote environment)' header comments.
Undo across an environment switch left the binding on the switched-to environment: the binding service's onDidRestore hook runs only on the first restore (didRunRestoreHooks), so a rerun restore after an undo rebuilt the environmentBindingKey projection at the cut point but never re-applied it to the live binding — and any path falling back to peekPersistedBinding would re-read the untruncated wire log and re-apply the newest (post-switch) binding, which is right for resume and wrong for undo. Register an AgentConversationUndoParticipant in the binding service: it reads the cut-point projection (or the seed binding when the cut predates the first binding event) and re-applies it with the full side effects — session workDir, background reconnect, environment reminder on a machine identity change, and the onDidChange fire so the footer slot follows. The resume path (hook + peek fallback) is untouched and the two paths cannot fight: the hook is once-per-dispatcher, the participant runs only on undo. The mechanism is event-general, so M49's model-initiated EnvironmentSetBinding ops ride the same revert path.
The dialogs now invoke the host's requestRender after async state changes; the command-flow test's host mock predates the method and crashed the switch/reconnect/add flows silently.
- Drop the last two stale '(experimental remote environment)' comments (commands/environment.ts header, footer environment slot). - Remove the dead ENVIRONMENT_UNAVAILABLE entry from the declare route's OpenAPI errors block — nothing in the declare handler throws it. - Brace the two new void-expression arrows (program.ts trust re-fire, remoteEnvironmentProvider trust listener) and drop the four unnecessary WireRecord assertions in the undo-test stub, restoring the lint warning count to baseline. - Correct the switch endpoint's cwd docs (en+zh): non-local switches require cwd (40001 when missing); the entry's defaultCwd applies only in the createSession seed path.
Four no-confusing-void-expression warnings from the Bug A wiring — same shape as the program.ts and remoteEnvironmentProvider.ts arrows braced in the previous sweep.
The remote-environment footer slot rendered the connecting status as static warning-colored text. Reuse the shared braille frame set and 80ms interval (the same set the thinking indicator uses) to tick a spinner ahead of the environment id. The timer is strictly bounded to the connecting status: it starts when the slot enters connecting and stops the moment the status moves on (or the footer is disposed), repainting each frame through the standard onRefresh path. Other statuses render exactly as before and the local environment still renders nothing.
…stubs - merge the glob/grep rgUnavailableGuidance suites into one parameterized suite - consolidate environmentBindingService setup/undoSetup onto shared stub builders - unify sessionManager remote-wiring setups (remoteWiringSetup/localRegistry/workspacesFor) - add test/environment/stubs.ts (stubAgentEnvironment/fakeEnvironment/connectableEnvironment) and adopt across tool, binding, registry, view, and sessionManager suites - share recordingAppendFs across task persistence and output-access suites - hoist EnvironmentRegistry construction in environmentRegistry.test.ts - extract resolve() helper in environmentDeclarations.test.ts and programWorkspace/noopLogger reuse in program.test.ts
- share expectCurrentBinding in agentLifecycle remote binding tests - extract stubHostProcess into test/os/stubs.ts for gitContext/spawn suites - wrap stubPlanEnvironment with the shared stubAgentEnvironment builder - fold the remote plans-dir mkdir assertion into the stores-plan test - route sessionInit environment stub through stubAgentEnvironment - share failingRunner across externalHooksRunner error-reporting tests - extract registerHookTestServices for the externalHooks integration tests - extract connectingReadExecution in read.test.ts - extract provideHost in environmentUnitHost.test.ts - dedup the image-originals fake fs builders - share resolverFor in fsService.test.ts and stubRgProbe across rgLocator suites
…m rule) Each deleted test pinned a behavior already covered by a surviving test on the same code path with the same inputs and oracle; every seam (DI registration, lifecycle wiring, reconnect/reroot orchestration) keeps its own pin: - binding 'acquires a ready environment without waiting on a readiness signal': survives in environmentRegistry 'acquires a ready environment through acquireWhenReady without waiting' (same registry path; strengthened with a never-resolving whenReady) + binding 'waits for the in-flight connect' (pins the non-turn service delegation of the same method) - binding 'rejects with the connect reason when the in-flight connect fails': survives in environmentRegistry 'rejects acquireWhenReady with the connect failure reason when the readiness signal rejects' (same path, same rejects.toBe(failure) oracle) - binding 'keeps the immediate environment.unavailable error for a plainly disconnected environment': survives in environmentRegistry 'keeps the immediate unavailable error on a plainly disconnected environment' (same path, adds the missing-env case) - sessionManager 'aborts creation when the configured default environment fails to connect': survives in sessionManager 'aborts creation when the environment connect fails' (same create->connect-failure path, stronger oracle) + 'connects the configured default environment before creating the session' (default resolution) - binding 'does not reconnect a replayed remote binding for the main agent': survives in agentLifecycle 'restores the main agent binding without the binding service reconnecting it' (same restore-hook path through the lifecycle DI seam, same no-connect oracle) - binding 'background-reconnects and reroots a replayed remote binding for non-main agents': survives in agentLifecycle 'restores a remote-bound subagent from wire records and background-reconnects' (same path through the lifecycle DI seam, same connectCalls/rerootCalls oracles) - binding 'keeps a gone declaration bound after reseed and fails explicitly at use': survives in agentLifecycle 'keeps a restored gone environment declaration bound and fails explicitly at use' (same behavior through the lifecycle DI seam) + environmentRegistry 'does not fallback when a environment is missing' (not_found at the registry layer)
…ng suites - read-media environmentFor, apply-profile mappedEnvironmentService, and the agentsMdReminder inline service now wrap stubAgentEnvironment (with an isAvailable/onDidChange option) - hoist the repeated GlobTool construction into beforeEach in glob.test.ts
the 23 identical manager.dispose()/registry.dispose() pairs in the remote environment wiring describe collapse into one afterEach, which also disposes on failure paths the inline calls never reached
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ec7e2e7e8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -123,7 +123,7 @@ export async function registerApiV1Routes( | |||
| core, | |||
| { sessionEventCursor: (sessionId) => opts.broadcaster.getCursor(sessionId) }, | |||
| ); | |||
| registerRuntimeRoutes(apiV1 as unknown as Parameters<typeof registerRuntimeRoutes>[0], core); | |||
| registerEnvironmentRoutes(apiV1 as unknown as Parameters<typeof registerEnvironmentRoutes>[0], core); | |||
There was a problem hiding this comment.
Keep the published runtime endpoints available
Existing REST clients that call GET or POST /api/v1/sessions/{id}/runtime now receive a 404: this replaces the runtime route registration, while the replacement registers only the new environment GET routes. Those endpoints were part of the documented API, so retain compatibility aliases (or version the API) before removing them; otherwise already released desktop/web and direct REST clients cannot read or switch their bindings.
AGENTS.md reference: AGENTS.md:L111-L113
Useful? React with 👍 / 👎.
| async getEnvironment(): Promise<AgentEnvironmentBinding> { | ||
| this.ensureOpen(); | ||
| return this.rpc.getRuntime({ sessionId: this.id }); | ||
| return this.rpc.getEnvironment({ sessionId: this.id }); | ||
| } |
There was a problem hiding this comment.
Retain Session runtime methods as compatibility aliases
SDK callers compiled against the previous release use session.getRuntime() and session.switchRuntime(runtimeId), but this replacement exposes only getEnvironment() and removes the runtime binding type and switch method. Updating to this release therefore breaks existing SDK integrations at compile time (and removes their switching capability); preserve deprecated runtime aliases or make this an explicitly versioned breaking release.
AGENTS.md reference: AGENTS.md:L111-L113
Useful? React with 👍 / 👎.
| Read a text file from the local filesystem. | ||
|
|
||
| The path may be a `kimi-file://` attachment reference. Its bytes come from the current session's storage, independently of the workspace runtime. Next Read keeps the reference so pagination also works after a fork. For a binary attachment, the error includes a server-local path when available; a converter must be able to access that filesystem. ReadMediaFile accepts the same reference for images and videos. | ||
| The path may be a `kimi-file://` attachment reference. Its bytes come from the current session's storage, independently of the workspace environment. Next Read keeps the reference so pagination also works after a fork. For a binary attachment, the error includes a server-local path when available; a converter must be able to access that filesystem. ReadMediaFile accepts the same reference for images and videos. |
There was a problem hiding this comment.
Describe Read as targeting the bound environment
For every remote-bound session, ReadTool now resolves its filesystem from the active environment, but its prompt still begins by telling the model it reads from the “local filesystem.” This description is delivered to the very agents that must operate on remote target paths, so it conflicts with the binding reminder and can lead them to give incorrect path guidance; describe the bound environment/target filesystem instead.
AGENTS.md reference: AGENTS.md:L107-L109
Useful? React with 👍 / 👎.
| `resume could not connect environment ${boundEnvironmentId}; session ${sessionId} cannot be loaded until the environment is available`, | ||
| { error }, | ||
| ); | ||
| throw error; |
There was a problem hiding this comment.
Preserve disconnected bindings when resuming
When a persisted remote environment is offline, this catch rethrows the connection error instead of materializing the session with its saved disconnected binding. SessionManager.resume therefore rejects and the TUI reports “Failed to resume session,” even though the new remote-environment guide promises that the session remains open and its first tool call retries; users cannot inspect or replay an existing remote session during an outage.
Useful? React with 👍 / 👎.
- drop protocol fields no client sends (read/write followSymlinks, createdAtMs, pipeStdin) and their server branches - remove product code only referenced by tests: Program.suggestFiles, Program.onDidChangeTrust, MCP originalsDir, kap-server resolveWorkspaceInstance, unused SlashCommandHost/KimiTUI members - merge duplicated logic: connect+acquireWhenReady into EnvironmentRegistry, prompts/skills media targets into environmentMediaTargets, realpath-existing-prefix into one helper, decodeBase64 reuse, single rg target mapping, node-sdk declarations via IEnvironmentDeclarationService - simplify fireAndForgetTrigger, sendWrite, inline launcherLabel and environmentEntryType, drop redundant exports EnvironmentRegistry.acquireWhenReady now triggers connect like the agent-level path already did; the two providerFactory tests pin the immediate post-failure error via acquire() instead.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5d9755637
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| try { | ||
| stat = await this.hostFs.stat(root); | ||
| } catch (error) { | ||
| const code = (unwrapErrorCause(error) as NodeJS.ErrnoException | undefined)?.code; | ||
| if (code === 'ENOENT' || code === 'ENOTDIR') { | ||
| throw new Error2(ErrorCodes.FS_PATH_NOT_FOUND, `workspace root ${root} does not exist`); | ||
| } | ||
| throw error; | ||
| } catch { | ||
| stat = undefined; | ||
| } |
There was a problem hiding this comment.
Reject invalid local workspace roots
When an SDK caller supplies a missing workDir (or a path below a regular file), this catch converts the failed local stat into a successful workspace registration; SDKRpcClientV2.createSession forwards that value to ISessionManager.create without another filesystem check. Before this change, createOrTouch returned fs.path_not_found, so callers can now receive and persist a session/workspace that only fails later when it first accesses the filesystem. Keep validating the local workspace root at the SDK/session boundary while allowing environmentCwd to be the separate remote target path.
AGENTS.md reference: AGENTS.md:L99-L101
Useful? React with 👍 / 👎.
| cwd: z.string().optional(), | ||
| executor: z.enum(['local', 'kaos']).optional(), | ||
| runtime_id: z.string().min(1).optional(), | ||
| environment_id: z.string().min(1).optional(), |
There was a problem hiding this comment.
Preserve the
runtime_id MCP configuration alias
Existing mcp.json stdio entries using runtime_id are parsed through McpServerConfigSchema.parse in configLoader.ts, but this replacement leaves that legacy key unknown and Zod strips it. McpConnectionManager then falls back to local, so an MCP server that previously ran in a configured remote runtime silently starts on the host after upgrade. Accept or migrate runtime_id as a deprecated alias (or make this an explicit breaking release) rather than changing the execution target without an error.
AGENTS.md reference: AGENTS.md:L119-L121
Useful? React with 👍 / 👎.
| @@ -6,7 +6,7 @@ import { | |||
| } from '@moonshot-ai/agent-core-v2/os/interface/terminal'; | |||
|
|
|||
| export const createTerminalRequestSchema = engineCreateTerminalRequestSchema.extend({ | |||
| runtime_id: z.string().min(1).optional(), | |||
| environment_id: z.string().min(1).optional(), | |||
There was a problem hiding this comment.
Preserve
runtime_id for terminal creation requests
Previously released clients send runtime_id to POST /sessions/{id}/terminals; the replacement schema strips that now-unknown field, and registerTerminalsRoutes defaults the missing environment_id to local. A client requesting a terminal in a non-local runtime therefore receives a successful terminal on the host instead of an incompatibility error or the requested target. Continue accepting runtime_id as an alias until the REST API is versioned.
AGENTS.md reference: AGENTS.md:L119-L121
Useful? React with 👍 / 👎.
| const result = z.object({ environment_id: z.string().min(1).optional() }).passthrough().safeParse(req.body ?? {}); | ||
| if (!result.success) { | ||
| reply.send(errEnvelope(ErrorCode.VALIDATION_FAILED, 'request body must be an object', req.id)); | ||
| return; | ||
| } | ||
| const { runtime_id, ...request } = result.data; | ||
| const runtimeId = runtime_id ?? 'local'; | ||
| const { environment_id, ...request } = result.data; | ||
| const environmentId = environment_id ?? 'local'; |
There was a problem hiding this comment.
Preserve
runtime_id for session filesystem requests
Previously released clients select a non-local target for every session filesystem action with runtime_id. This new selector leaves that key as ordinary request data and defaults the missing environment_id to local; the per-action Zod schemas then discard runtime_id. A remote fs:read, fs:mkdir, search, or git request can therefore succeed against the host filesystem after upgrade. Accept runtime_id as a deprecated alias or reject it explicitly instead of silently changing the target.
AGENTS.md reference: AGENTS.md:L119-L121
Useful? React with 👍 / 👎.
| sessionId: input.id, | ||
| workDir, | ||
| additionalDirs: input.additionalDirs, | ||
| environmentId: input.environmentId, | ||
| environmentCwd: input.environmentCwd, |
There was a problem hiding this comment.
Await remote provider registration before session creation
When an SDK caller constructs SDKRpcClientV2 and immediately creates a session using a declared remote environment (or the configured default), provider attachment can still be waiting on the environment/config readiness path. This call then reaches ISessionManager.create before the declaration has been registered, so the valid environment is reported unavailable; retrying later succeeds. Await remoteEnvironmentProvider before resolving or creating an environment-bound session so configured targets do not fail based on timing.
AGENTS.md reference: AGENTS.md:L99-L101
Useful? React with 👍 / 👎.
Released clients and mcp.json entries that only set runtime_id otherwise silently fall back to local after the runtime-to-environment rename. Cover MCP stdio config, terminal creation, and the session fs/search/suggest/download routes; environment_id wins when both keys are present.
…session use ISessionManager.create again rejects a missing or non-directory workDir with fs.path_not_found for local bindings; remote bindings keep deferred root validation, and createOrTouch stays lenient. SDKRpcClientV2 awaits the remote environment provider attach before create/resume/fork/reload and temporary resume, so declared or persisted remote environments no longer report unavailable or fall back to a local controller based on registration timing.
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
Cold resume reads the binding from state.json instead of scanning the agent wire. An unregistered or unreachable remote binding fails the resume instead of opening a local controller. Provider attach is part of EnvironmentService.ready, so create and resume wait in one place.
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1a438007d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -1,5 +1,5 @@ | |||
| export { KimiHarness } from '#/kimi-harness'; | |||
| export type { KimiHarnessRuntimeOptions } from '#/kimi-harness'; | |||
| export type { KimiHarnessEnvironmentOptions } from '#/kimi-harness'; | |||
There was a problem hiding this comment.
Preserve KimiHarnessRuntimeOptions as a compatibility alias
SDK consumers that import the previously published KimiHarnessRuntimeOptions type from the package root will fail to compile after this release, even though the options remain structurally compatible under the new name. Keep the old exported name as a deprecated alias (or make this an explicit major-version break) so existing SDK integrations can upgrade without source changes.
AGENTS.md reference: AGENTS.md:L111-L113
Useful? React with 👍 / 👎.
| env: StringRecordSchema.optional(), | ||
| cwd: z.string().optional(), | ||
| executor: z.enum(['local', 'kaos']).optional(), | ||
| environment_id: z.string().min(1).optional(), |
There was a problem hiding this comment.
Reject legacy
executor: 'kaos' configurations
When an existing mcp.json or inline MCP configuration sets executor: 'kaos', the default Zod object behavior strips it here, and the workspace MCP manager resolves no explicit environment to its local binding. Before this change that value was explicitly rejected as not_implemented; now the configured command starts on the host without an error, changing its execution target. Preserve a validation failure for the removed value (or migrate it) rather than silently discarding it.
AGENTS.md reference: AGENTS.md:L119-L121
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60d3b925b4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const create = () => this.controllerForWorkspace(workspace.id).create(options); | ||
| const create = async () => { | ||
| if (environmentId !== undefined) await this.connectForCreate(environmentId, environmentCwd); | ||
| const controllerEnvironmentId = environmentId ?? LOCAL_ENVIRONMENT_ID; |
There was a problem hiding this comment.
Reject invalid default environment declarations
When a session relies on [environments].default and any environment declaration is invalid (for example, a missing host or an invalid default target), ConfigService drops the entire section as a warning. declarations?.default is then absent and this fallback selects local, so commands and writes run on the host despite the user configuring a remote default. Fail creation when the environments section was rejected instead of treating it as no default.
Useful? React with 👍 / 👎.
| const fs = local.fs!; | ||
| const state = own(new WorkspaceStateService(this.dependencies.appState)); | ||
| const trust = own(new WorkspaceTrustService(this.context, this.dependencies.docs, state, this.dependencies.telemetry, this.dependencies.bootstrap)); | ||
| const mcpConfig = own(new WorkspaceMcpConfigService(this.context, this.dependencies.bootstrap, this.dependencies.plugins, this.dependencies.log, this.dependencies.config, fs, trust, this.dependencies.configStore)); | ||
| const mcp = own(new WorkspaceMcpService(this.context, this.resolver, mcpConfig, this.dependencies.oauth, this.dependencies.log, this.dependencies.telemetry, this.dependencies.identity, this.dependencies.sessionManager)); |
There was a problem hiding this comment.
Load project MCP configuration from the bound environment
For a remote-bound session, createGeneration() creates target-specific filesystem, instructions, and skills, but every generation receives this shared MCP configuration built from local.fs and the local workspace context. As a result, a target project's .kimi-code/mcp.json is never loaded, even though the remote-environment guide says target MCP configuration is loaded at session start; construct the MCP config per environment/cwd or load the target file before sharing the manager.
Useful? React with 👍 / 👎.
| this.entries.delete(environmentId); | ||
| this.changeEmitter.fire({ environmentId }); | ||
| await this.retire(previous); |
There was a problem hiding this comment.
Drain removed environments before disposing their resources
When config watching removes an environment while remote Bash or MCP work is running, remove() immediately invokes retire, which disposes tracked processes and the shared environment connection; the executor then terminates the active request. This contradicts the documented drain behavior that existing sessions retain the connection until in-flight work releases, so a configuration edit can kill an in-progress turn; retain the removed entry until its tracked resources drain.
Useful? React with 👍 / 👎.
| await target.fs.appendText(join(target.dir, `${taskId}.log`), chunk); | ||
| } catch {} | ||
| return target.dir; |
There was a problem hiding this comment.
Don't advertise a spill file after its remote write fails
When the target filesystem becomes unwritable or disconnects while a remote task is producing output, appendText throws here, but the catch still returns target.dir. AgentTaskService persists that directory as outputSpillDir, and subsequent TaskOutput/Wait results instruct the agent to Read a remote log file that was never written, rather than exposing the durable server-side output. Return no spill directory on failure (and keep reporting the server-local path) so the advertised output path is usable.
AGENTS.md reference: AGENTS.md:L119-L121
Useful? React with 👍 / 👎.
| const resolved = options.environmentId === undefined ? declarations?.default : undefined; | ||
| const environmentId = options.environmentId ?? resolved?.environmentId; | ||
| const environmentCwd = options.environmentCwd ?? resolved?.cwd ?? declared?.entry.defaultCwd; | ||
| const effective = | ||
| environmentId === undefined && environmentCwd === undefined | ||
| ? options | ||
| : { ...options, environmentId, environmentCwd }; |
There was a problem hiding this comment.
Resolve additional directories against the bound target
When a remote session is created with --add-dir (or SDK additionalDirs), this keeps the caller's local workDir while attaching a remote environmentCwd. SessionLifecycleService.materializeSession subsequently calls workspaceDirs.mergeAdditionalDirs(opts.workDir, ...) using the target filesystem, so relative paths resolve from the host workspace and absolute host paths either fail validation or select an unintended target directory. Use the effective remote cwd when merging extra directories for a non-local environment.
Useful? React with 👍 / 👎.
- remove task-output spill pinning/persistence (only meaningful once runtime rebinding lands) - resolve the main-agent environment inside session lifecycle via a new ensureMainAgent create option instead of re-resolving declarations in the kap-server sessions route - revert workspace trust onDidChange to a plain Emitter (no waitUntil consumer exists) - drop unused TerminalSpawnOptions.env and the dead Program.sessionControllerGeneration getter - inline single-use wrappers (suggestFiles helpers, originals/spill targets, locateSession options object) - reuse environmentStatusSchema in the REST environment protocol - simplify the exec-server entry's exit handling
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback". |
- drop tests whose turn-event fixtures no longer reach the binding service (it never subscribes), and their duplicated mid-turn cases - drop tautological replace-vs-timer races and tests fully covered by stronger pins elsewhere (guidance messages, pending-drain, initialize timeout, etag/range, node-walk fallback) - merge parallel bashTool/globTool test files into the existing tool test files per repo convention - collapse identical expensive e2e runs (argv spellings, transports) - assert overlay resource grouping instead of lease.track call counts
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 233b7723d7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| export class EnvironmentSetBinding extends AgentEvent2<z.infer<typeof environmentSetBindingSchema>> { | ||
| static override readonly type = 'environment.set_binding'; | ||
| static override readonly durable = true; | ||
| static override readonly schema = environmentSetBindingSchema; |
There was a problem hiding this comment.
Migrate legacy runtime-binding journal records
Sessions written by the previous release after session.switchRuntime(...) contain durable runtime.set_binding records, but this replacement registers only environment.set_binding. Restore skips unregistered record types, so a formerly non-local binding is lost and the main agent starts on local, causing its file and process tools to run on the host without an error. Recognize/migrate the legacy record (or fail the resume) rather than silently changing the execution target for persisted SDK sessions.
AGENTS.md reference: AGENTS.md:L113-L113
Useful? React with 👍 / 👎.
Requirement or Bug
支持将 agent 会话在创建时绑定到 SSH 或 Docker 执行环境。本 PR 是 #3856 的 phase 1,不包含会话内环境切换。
Bug Reproduction Steps
N/A
Root Cause
N/A
Code Changes
ACP 仍保留窄化后的
bind(environmentId, cwd):ACP client terminal 是进程内动态注册的 session environment,new/load/resume/fork 都需要把 main agent 绑定到该环境。不在本 PR 的 phase 2 surface:
change_environment/connectagent tools、Agent tool 的environment参数、/environment管理器、REST/SDK switch 与 declare endpoints,以及 binding switch undo。Behavior Changes and Affected Users
[environments]、--environment或 session-create 参数绑定 SSH/Docker/command 环境--environment/newdisconnected和连接错误,不额外写 transcript notice;下一次工具调用尝试重连ssh_hosts/sshHosts受影响模块与验证:
agent-core-v2:环境 registry/binding、远程协议、session/subagent 生命周期、Plan 文件恢复;本轮 focused tests 368 项通过。apps/kimi-code:启动、/new、footer 与失败进度;本轮 focused tests 417 项通过。acp-server:动态 session environment binding 保持不变;149 项测试通过。node-sdk:session 环境状态与生命周期;71 项测试通过。kap-server:环境 REST route 与 API snapshot;10 项测试通过。agent-core-v2、kap-server、node-sdk、klient、acp-server、apps/kimi-codetypecheck 通过;repository lint 与 no-comments check 通过。已知待 undraft 前处理:其余
docs/与现有 changesets 仍有来自 #3856 的 phase-2 描述,需要按 phase 1 重新收口;EnvironmentBinding.cwd尚未在 bind 时 canonicalize。Checklist
gen-changesetsskill; no additional changeset was added. The fail-closed resume and session-meta binding are unreleased phase-1 behavior, not a change from the last release.gen-docsskill; the English and Chinese server API references match the phase-1 response shape.